feat(newsletter): newsletter status handling#2435
Open
Maciej D (mdanilowicz) wants to merge 1 commit intomainfrom
Open
feat(newsletter): newsletter status handling#2435Maciej D (mdanilowicz) wants to merge 1 commit intomainfrom
Maciej D (mdanilowicz) wants to merge 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes newsletter subscription flows by syncing useNewsletter’s reactive status from the subscribe API response, and removing follow-up newsletter status refresh calls in account overview pages.
Changes:
- Update
useNewsletter.newsletterSubscribe()to setnewsletterStatusdirectly from the subscribe response. - Remove redundant
getNewsletterStatus()refresh calls after subscribe actions invue-starter-templateandvue-demo-storeaccount overview pages. - Add changesets for
@shopware/composables,vue-demo-store, andvue-starter-template.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/vue-starter-template/app/pages/account/index.vue | Removes post-subscribe status refresh call. |
| templates/vue-demo-store/app/pages/account/index.vue | Removes status refresh (and related UI state sync) in finally. |
| packages/composables/src/useNewsletter/useNewsletter.ts | Syncs newsletterStatus from subscribe API response. |
| .changeset/swift-demo-store-newsletter.md | Patch note for demo store change. |
| .changeset/sharp-eagles-sync.md | Patch note for composables change. |
| .changeset/kind-starter-template-newsletter.md | Patch note for starter template change. |
Comments suppressed due to low confidence (1)
packages/composables/src/useNewsletter/useNewsletter.ts:85
newsletterSubscribenow updatesnewsletterStatusfrom the subscribe response, butnewsletterUnsubscribestill leavesnewsletterStatusunchanged. This can leaveisNewsletterSubscriber/confirmationNeededstale after an unsubscribe (and prevents safely removing follow-up status refreshes). Consider updatingnewsletterStatusinnewsletterUnsubscribeas well (e.g., set it tooptOuton success or update from an API response/status call).
newsletterStatus.value = result.data.status;
return result.data;
}
async function newsletterUnsubscribe(email: string) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
63
to
65
| } finally { | ||
| await getNewsletterStatus(); | ||
|
|
||
| newsletter.value = isNewsletterSubscriber.value; | ||
| newsletterDisabled.value = false; | ||
| } |
Comment on lines
+81
to
82
| newsletterStatus.value = result.data.status; | ||
| return result.data; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #2327
This pull request streamlines the newsletter subscription logic by ensuring that the
useNewslettercomposable updates the subscription status directly from the API response. As a result, redundant status refresh calls in the account overview pages are removed, making the code more efficient and reducing unnecessary API requests.Newsletter subscription logic improvements:
useNewsletterso thatnewsletterStatusis set directly from the subscribe API response, keepingisNewsletterSubscriberin sync without requiring an extra status request (useNewsletter.ts). [1] [2]Redundant code removal in account overview pages:
vue-starter-templateaccount overview page, since state is now updated by the composable (index.vue). [1] [2]vue-demo-storeaccount overview page for the same reason (index.vue). [1] [2]